-
-
Notifications
You must be signed in to change notification settings - Fork 477
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: detectBrowserLanguage overrides non-root paths #603
fix: detectBrowserLanguage overrides non-root paths #603
Conversation
Codecov Report
@@ Coverage Diff @@
## master #603 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 3 3
Lines 121 121
Branches 31 31
=====================================
Hits 121 121 Continue to review full report at Codecov.
|
|
Yes, I'll try to write a few tests.
I don't think so, as the url triggers the page to be viewed. When using the no_prefix strategy, nuxt-i18n still generates all routes per language correctly. When requesting the url of a certain language, even without the prefix, the current mechanism should work without any extra effort, as it exists and is not touched. Of course, it will not redirect to the browser language, but I think that's the whole point - browser language is not always the language required by the user. Even when it is, redirecting seems like "too much magic". But maybe that can be a choice of the developer, when setting up the webapp. Example scenarios where this might be unexpected, or even work against the end user:
IMO, to always redirect to the browser language or the cookie value previously stored only makes sense when users are requesting the the root url. It is generic and they don't yet know what they are searching for. Any other url should be treated as-is. Its up to the developer to create a language-switching mechanism (menu, button, dropdown, etc).
Of course. |
Sorry for not getting back to you earlier. There was so much text that I was delaying answering this forever. :) So you seem to be saying that it will work and then you are also saying later that it won't ( But to clarify things that I think you might be wrong about (or maybe I'm understanding you wrong).
With
With So this change is changing (you could argue whether breaking) language detection when going to such URL. If one goes to You can test that by:
With the change it will be English while without french. And as for the rest of your comment...
Is it really more relevant example than a user using his own computer? The latter seems a more likely and common scenario. Shouldn't we cater for majority if we can't satisfy both at the same time?
That seems like a valid point. If the user went to the page with specific language prefix and got redirected to a non-existing page then it would be bad. But what about no-prefix strategy, where URLs are the same for every language? Should we treat those differently then? If the user goes to site.com/product-name then it seems like it would make sense to show him that page in his language of choice. Why should we then treat that URL differently than root URL and default to english? |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
I like the change, just need to work on tests and changing behavior a bit, according to the comments. I'll work on that later. |
Check out the discussion in #761 and my proposed solution: bitbuyAT@6228014 I'm not very familiar with the nuxt-i18n code, so my solution may not be ideal or work for every single configuration/strategy, but by adding a In a later version you could just change the default value for |
I'll handle this when I have some time but my earlier comments still apply - that solution wouldn't be right for |
I'm still unsure why you're still talking about It simply doesn't make sense.
If there is no locale prefix added - it's not even possible to detect locale from route. The point here is prefix_and_default where locale can be detected from route by passing browser language auto detection or redirection. Put it simply, whenever I share http://examples/fr/article language should be set to French even if the person browser language is Arabic. That's the point. Bots can't change cookies and we should just add option to force changing language based on prefixes. Bot visits:
Thanks! |
This is not about detecting locale from route but from saved cookie or browser-supported-locales (reported through HTTP header or [I guess I wasn't clear before on what I mean by "redirecting". I meant it in the sense of changing the language of the page (with or without redirecting to another URL)] If the user visits the prefixed route then I agree we should not redirect to another language. But when the user visits a non-prefixed route (index or any route when using The proposal here is to redirect when using strategies with prefixes. That's sort of OK but google will still have issue with that one page redirecting. When using |
This is the main problem and still I don't see why we are stuck at
Can't we just apply a "fix" only when prefix_and_default used with additional configuration without breaking up anything like what other libraries are doing? Thanks! |
I'm pretty sure the workaround I posted would also work for Long to medium term I think there should be some discussion about nuxt-i18n default strategy and behaviour with a focus on SEO (we all try build our websites to be found after all). Because currently (without workarounds) the only way get your non-english pages indexed is by not using redirects at all. |
Your solution would disable locale detection for non-root paths. And is it correct to disable language detection for a route like But it's easy enough to exclude |
Ok, I think I misunderstood how the But either way, all my solution does is add an option. All settings unchanged, nuxt-i18n would behave exactly as it does now. Behaviour would only change when enabling the |
It doesn't make sense to use Thanks! |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Any updates on this? Looking forward for it :) |
A variant of this fix was just merged through #799. |
This commit tries to fix issue #455.
With the fix:
Solution provided by @konr4d. I'm just doing the PR to speed up the process :)